Skip to content

Improve facet error messages in animint2dir#285

Open
ANAMASGARD wants to merge 8 commits into
masterfrom
fix-168-facet-error-message
Open

Improve facet error messages in animint2dir#285
ANAMASGARD wants to merge 8 commits into
masterfrom
fix-168-facet-error-message

Conversation

@ANAMASGARD
Copy link
Copy Markdown
Contributor

@ANAMASGARD ANAMASGARD commented Dec 21, 2025 •
edited
Loading

FIXES #168

What's broken

viz <- list(
  plot = ggplot() + 
    facet_wrap(. ~ Species) +  # formula notation
    geom_point(aes(x, y), data = iris)
)
animint2dir(viz)
  • Current error:
    Error: At least one layer must contain all variables used for facetting

Doesn't tell you what's wrong or how to fix it.

  • Workaround that works:
    facet_wrap("Species") # string notation works fine

What I am fixing

  1. Detect when formula notation is used in facets
  2. Give clear error telling user to use string notation instead
  3. Fix happens in animint2dir() - animint2-specific code only

Next commit will add the fix to make tests pass.

@ANAMASGARD
test: add failing tests for facet error messages
259505a 
Issue #168 - facet_wrap(. ~ var) gives unhelpful error when var is missing.

Added tests that will pass once we improve error messages in animint2dir().
Tests cover facet_wrap and facet_grid with missing variables.

These tests are animint2-specific .
@ANAMASGARD
fix: improve error message for missing facet variables
2472155 
When facet_wrap or facet_grid uses formula notation with a variable
that does not exist in the data, the error now shows:
- Which variable is missing
- What columns are available
- Suggests using string notation instead

Fixes #168
@ANAMASGARD
Copy link
Copy Markdown
Contributor Author

ANAMASGARD commented Dec 22, 2025 •
edited
Loading

Before :-

Error: At least one layer must contain all variables used for facetting

After :-

Error: Facet variable not found in data: MissingVar
Available columns: Sepal.Length, Sepal.Width, Petal.Length, Petal.Width, Species
Use string notation like facet_wrap("var") instead of formula notation facet_wrap(. ~ var) ```

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 22, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.37%. Comparing base (535ded3) to head (c1877ef).

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
+ Coverage   73.34%   73.37%   +0.02%     
==========================================
  Files         164      164              
  Lines        8861     8870       +9     
==========================================
+ Hits         6499     6508       +9     
  Misses       2362     2362
Flag Coverage Δ
javascript 81.23% <ø> (ø)
r 69.67% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tdhock
Copy link
Copy Markdown
Collaborator

tdhock commented Dec 22, 2025 •
edited
Loading

No obvious timing issues in HEAD=fix-168-facet-error-message
Comparison Plot

Generated via commit c1877ef

Download link for the artifact containing the test results: ↓ atime-results.zip

Task Duration
R setup and installing dependencies 2 minutes and 12 seconds
Installing different package versions 27 seconds
Running and plotting the test cases 3 minutes and 29 seconds

@ANAMASGARD
Copy link
Copy Markdown
Contributor Author

ANAMASGARD commented Dec 22, 2025

Sir @tdhock The 2 failing tests in JS_coverage are in test-compiler-ghpages.R (GitHub Pages tests), not related to this PR. This is the same race condition issue where parallel CI jobs conflict over the shared test repository.

All facet-related tests pass:

  • R_coverage: 2096+ tests passed
  • CRAN: passed
  • My 3 new facet error message tests pass

The fix is working correctly. Please review and give your feedback Sir .

@ANAMASGARD ANAMASGARD requested a review from tdhock December 22, 2025 10:04
tdhock
tdhock requested changes Feb 27, 2026
View reviewed changes
Comment thread tests/testthat/test-renderer-facet-error-messages.R Outdated
Comment thread R/z_animintHelpers.R Outdated
@ANAMASGARD ANAMASGARD requested a review from tdhock March 18, 2026 03:30
tdhock
tdhock requested changes May 8, 2026
View reviewed changes
Comment thread R/facet-layout.r
# variables that appear in the data
has_all <- unlist(plyr::llply(values, length)) == length(vars)
if (!any(has_all)) {
vars_to_check <- vars[vars != "."]
Copy link
Copy Markdown
Collaborator

@tdhock tdhock May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setdiff?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdhock tdhock tdhock requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

facet_wrap ~var errors but "var" works

2 participants

@ANAMASGARD @tdhock